Repository navigation
codegen: an unsigned bound absorbs the element key's sign test (N10) - #14426
Conversation
…n a leaving path keeps the narrowed reads (R1 range tightening)
…s its element base (P4 lever 1, PR-D)
…nts is no coercion; the arrbase fixture grows the board by pushes
…ve the held base)
…aving-writes # Conflicts: # crates/perry/src/commands/compile/compile_env/tests.rs
…and the admission fixture's loop is not unrolled
# Conflicts: # crates/perry-codegen/src/expr/index_get.rs
…ameter bound is a coercion ender)
…nter key goes to the index method clone)
…s' into perf/unsigned-bound # Conflicts: # scripts/test-verify-facts-s0.py
An element read through the canonical key split tested the key's sign and then bounded it with an unsigned compare. The tier's bound already declines a negative key (it is at least 2^31 unsigned), so the sign test moves to the tier's miss arm: a negative key leaves there for the exact property-key arm, a non-negative miss keeps the tier's cold read. A widened native key is no longer mapped to -1 by a select, on reads and on the guarded stores. Reads whose key is proven in range have no bound and keep the split's test. PERRY_UNSIGNED_BOUND=0 turns it off (a compile-cache input). Check mode tests that only non-negative keys reach a tier's cold read (fact "unsigned bound"); PERRY_FACTS_SABOTAGE=unsignedbound drops the exit (S0 unsigned-bound-negative). Parity fixture test_unsigned_bound.ts covers -1, -0, NaN, 2^31, 2^32-1, 1.5, string keys, holes and length 0.
# Conflicts: # crates/perry-wpfacts/src/gen/walk.rs # crates/perry/src/commands/compile/compile_env.rs # crates/perry/src/commands/compile/compile_env/tests.rs # scripts/test-verify-facts-s0.py
# Conflicts: # scripts/test-verify-facts-s0.py
|
Caution Review failedThe pull request is closed.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
|
Re-measured on the final head fc5284f (this PR merged with origin/main 963ffae, which includes N2 #14424, N7 and VR-0). qb10, instructions:u, on vs
N10 composes with N2's function-level bases: the gain matches the rel2 table. The release build of this head is clean. The Supabase size census on this head (via |
|
Supabase CLI size on the final head fc5284f (qb10, via
It shrinks, as chess, skin and tssrc do. |
CENSUS-residual §5 N10, cause (i): an element read through the canonical key split tested the key's sign (
test; mov $-1; cmovns; js, ortest; jsafter the normalizer) and then bounded it with an unsigned compare. The bound already declines every negative key, because a negative i32 is at least 2^31 unsigned and every length, capacity and floor is below 2^31. This PR moves the sign test off the hit path and onto the miss arm. It is codegen only, and wpfacts is unchanged.D (#14420) and R1 (#14416) are both in main. The branch merges origin/main 682e548 (the measured head) and then 963ffae (N2 #14424 among others), and
git merge-treeagainst it is clean. N10 changes only the guard computation, so it composes with N2's function-level bases.What changes
expr/index_get/runtime_key.rs,lower_array_index_get_via_canonical_i32_split).guarded_array::unsigned_bound_covers), the split branches straight into the tier.idx <s 0 → runtime-key arm(guarded_array::emit_negative_exit, blockubound.cold). A negative key therefore still reaches the runtime-key arm with its original double (mode 0, the exact property-key read). A non-negative miss still reaches the tier's own cold read.idx <u prefixon the prefix hit,idx <u lengthandidx <u capacityon the full hit. Diagnostic builds with the observing guard are excluded, because the guard call receives the key.idx <u capacity);idx <u len;proven_array::bounds_key).sitofp/uitofp i32source:k | 0, an int32 local) is handed over as it is (index_key::normalize_unsigned), with noselect(idx >= 0, idx, -1). The two store paths that decline every rejected key onto one helper with the original double get the same:index_set.rs's runtime-key store andStoreIndex::CanonicalOfDouble. Their one bound isidx <u capacity. Every other key form keeps its admission: the normalizer call still returns-1for NaN, fractions, tags and out-of-range values,-0still maps to element 0, and thecharCodeAtalias keeps its ordered/pristine select.ArrayHeader.length/capacityareu32slots, and every element tier already compares them unsigned. A key at or above 2^31 (auitofpsource, or-1from the normalizer) is declined by the bound and is negative as i32, so it takes the runtime-key arm as before.The evaluate site from the disassembly (qb10, rel2,
evaluate$sig, one of the two normalizer sites):Sign-test census on the chess hot functions (
jscount in the disassembly, on vsPERRY_UNSIGNED_BOUND=0, qb10 rel2): evaluate 2 → 0, generatePseudo 1 → 0, make 25 → 9, negamax 20 → 12, quiescence 21 → 14,set$generic5 → 0, set 6 → 2, unmake unchanged.attackedhas no sign test on either arm: after D and R1 its reads are in the floor or held by a base. The remainingcmovnsin make are number-to-string sign handling (or $0x2d, the '-' character) and ToInt32's slow path, not element bounds.Check mode, switch, sabotage
PERRY_VERIFY_FACTS=1: the tier's cold read, after the negative exit, tests that its key is non-negative. That is the fold's claim that only non-negative keys reach it. The new fact is "unsigned bound" (verify_facts::Fact::UnsignedBound).PERRY_UNSIGNED_BOUND=0: every read and store keeps its sign test in front of the bound (today's code). It is a compile-cache input (compile_env.rs, testunsigned_bound_switch_is_a_cache_input).PERRY_FACTS_SABOTAGE=unsignedbound: the miss arm drops the negative exit. S0unsigned-bound-negative.tsexits 0 on the correct arm and 86 on the sabotaged arm ("unsigned bound").Results (instructions:u, on vs
PERRY_UNSIGNED_BOUND=0on the same binary, two runs each, identical; checksums equal)qb10, the head (rel2 = d4ca346: N10 merged with origin/main 682e548, which contains D, R1, N1a and N1b) (M):
qb10, the final head (rel3 = fc5284f: merged with origin/main 963ffae, which includes N2 #14424 and N7) (M; posted as a PR comment):
qb9, rel1 = 3fe1ade: N10 on main 0e35b90 with the D and R1 branches merged locally, before N1a/N1b (M; its own table):
-1select now goes away as well./usr/bin/timemax RSS, on / off): search 26 880 / 26 880 kB, perft, evaluate and legal 19 456 / 19 456 kB.Gates
PERRY_GC_MOVING_LOOP_POLLS=1, real GC stress on, off and in check mode = 133/133 MATCH (node v26.8.1). qb9 rel1: 17 fixtures, 119/119 MATCH (node v26.12.1). The fixtures are the newtest_unsigned_bound, R1'stest_range_tightening, D's loop_base/loop_base_gc, C3/C2/B's array-rep fixtures,test_gap_array_index_range_proofandtest_guarded_raw_numeric_arraysPERRY_GC_SCHEDULE_SEED=7at compile time,PERRY_GC_SCHEDULE_RATE=1at run time. Forced collections: test_unsigned_bound 8 (4 012 moved objects), loop_base_gc 25 000 (460 087 moved objects, 9 750 512 polls), floor_growth 11, range_tightening 7. The counts are the same with the switch offunsigned-bound-negative: correct 0, false 86. Every older arm is unchangedstring/test_issue_9810_virtual_string_indices, fails identically in both arms ("Error: custom prototype", pre-existing); only the code address in the printed stack frame differsPERRY_VERIFY_FACTS=1(qb9 rel1)test_gap_s8b_computed_fields,test_gap_10554_fn_identity_own_module)-D warningsscripts/check_file_size.sh-D warningsnum_seeds_audit2 passed; perrycompile_env7 passed (withunsigned_bound_switch_is_a_cache_input), 1 failed:codegen_env_vars_are_build_cache_inputs, main's known failure ("shared list must be sorted and unique", FOLLOWUPS); perry-hir 881 passed; perry-runtime 6654 passed, 0 failed-D warnings(qb10, 4ba7832)any_typed_dynamic_key_takes_the_numeric_tiers_when_it_is_an_array_indexpinned the split'sicmp sge, and 4ba7832 updates it to the miss-arm sign test (icmp slt,ubound.cold).-D warnings0 errors ×4The last head, fc5284f, merges origin/main 963ffae (N2 #14424, VR-0 #14423, N7, the Supabase top-level-await fix). The only conflict was the S0 arm list, which is additive. None of those commits touches this PR's files.
Size (every executable section; on vs off, qb9 rel1)
.textperry_js_textSupabase size: on rel1 (qb9) both arms stopped at the macro runner: main was broken from 8999cdb (exit 13, top-level await, #14413) until #14425. The row above is from the head rel3 (which contains 963ffae), compiled via
/root/supa-one.shon qb10; the other rows are from rel1 on qb9.Departures and what is not done
[0, floor), so the split's test is redundant there too, but removing it is the "interval proves ≥ 0" case: a different lever, not done here.at$idx_u31clones,Array.prototype.atand typed arrays are not touched. The clones take a raw non-negative i32 and have no split.at()with a negative index has different semantics (it counts from the end). The typed-array element paths have no sign test in front of their bound in the code here.